From 3924d67605bff2674c7294d03e9cc369bfc8cbee Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Mon, 2 Mar 2026 11:11:34 +0000 Subject: [PATCH 1/6] feat: add detailed logging and error handling to notification API views --- .../core/djangoapps/notifications/views.py | 708 ++++++++++++------ 1 file changed, 483 insertions(+), 225 deletions(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 091be365d45f..8f4991117dd9 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -1,6 +1,7 @@ """ Views for the notifications API. """ +import logging from datetime import datetime, timedelta from django.conf import settings @@ -15,12 +16,21 @@ from rest_framework.response import Response from rest_framework.views import APIView -from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch, username_from_hash +from openedx.core.djangoapps.notifications.email.utils import ( + update_user_preferences_from_patch, + username_from_hash +) from openedx.core.djangoapps.notifications.models import NotificationPreference -from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user +from openedx.core.djangoapps.notifications.permissions import ( + allow_any_authenticated_user +) -from .base_notification import COURSE_NOTIFICATION_APPS, NotificationAppManager, COURSE_NOTIFICATION_TYPES, \ +from .base_notification import ( + COURSE_NOTIFICATION_APPS, + NotificationAppManager, + COURSE_NOTIFICATION_TYPES, NotificationTypeManager +) from .events import ( notification_preference_update_event, notification_read_event, @@ -40,6 +50,8 @@ exclude_inaccessible_preferences ) +logger = logging.getLogger(__name__) + @allow_any_authenticated_user() class NotificationListAPIView(generics.ListAPIView): @@ -78,34 +90,58 @@ class NotificationListAPIView(generics.ListAPIView): def get_queryset(self): """ - Override the get_queryset method to filter the queryset by app name, request.user and created + Override get_queryset to filter by app name, user and created. """ - expiry_date = datetime.now(UTC) - timedelta(days=settings.NOTIFICATIONS_EXPIRY) - app_name = self.request.query_params.get('app_name') - - if self.request.query_params.get('tray_opened'): - unseen_count = Notification.objects.filter(user_id=self.request.user, last_seen__isnull=True).count() - notification_tray_opened_event(self.request.user, unseen_count) - params = { - 'user': self.request.user, - 'created__gte': expiry_date, - 'web': True - } + try: + expiry_date = datetime.now(UTC) - timedelta( + days=settings.NOTIFICATIONS_EXPIRY + ) + app_name = self.request.query_params.get('app_name') + + if self.request.query_params.get('tray_opened'): + unseen_count = Notification.objects.filter( + user_id=self.request.user, + last_seen__isnull=True + ).count() + notification_tray_opened_event( + self.request.user, + unseen_count + ) + params = { + 'user': self.request.user, + 'created__gte': expiry_date, + 'web': True + } - if app_name: - params['app_name'] = app_name - return Notification.objects.filter(**params).order_by('-created') + if app_name: + params['app_name'] = app_name + queryset = Notification.objects.filter( + **params + ).order_by('-created') + logger.info( + 'Retrieved notifications for user %s with app_name=%s', + self.request.user.id, + app_name, + ) + return queryset + except Notification.DoesNotExist as exc: + logger.error( + 'Failed to retrieve notifications for user %s: %s', + self.request.user.id, + str(exc) + ) + raise @allow_any_authenticated_user() class NotificationCountView(APIView): """ - API view for getting the unseen notifications count and show_notification_tray flag for a user. + API view for getting unseen notifications count and tray flag. """ def get(self, request): """ - Get the unseen notifications count and show_notification_tray flag for a user. + Get the unseen notifications count and show_notification_tray flag. **Permissions**: User must be authenticated. **Response Format**: @@ -123,268 +159,461 @@ def get(self, request): **Response Error Codes**: - 403: The requester cannot access resource. """ - # Get the unseen notifications count for each app name. - count_by_app_name = ( - Notification.objects - .filter(user_id=request.user, last_seen__isnull=True, web=True) - .values('app_name') - .annotate(count=Count('*')) - ) - count_total = 0 - show_notifications_tray = get_show_notifications_tray(self.request.user) - count_by_app_name_dict = { - app_name: 0 - for app_name in COURSE_NOTIFICATION_APPS - } + try: + # Get unseen notifications count for each app name. + count_by_app_name = ( + Notification.objects + .filter( + user_id=request.user, + last_seen__isnull=True, + web=True + ) + .values('app_name') + .annotate(count=Count('*')) + ) + count_total = 0 + show_notifications_tray = get_show_notifications_tray( + self.request.user + ) + count_by_app_name_dict = { + app_name: 0 + for app_name in COURSE_NOTIFICATION_APPS + } - for item in count_by_app_name: - app_name = item['app_name'] - count = item['count'] - count_total += count - count_by_app_name_dict[app_name] = count + for item in count_by_app_name: + app_name = item['app_name'] + count = item['count'] + count_total += count + count_by_app_name_dict[app_name] = count - return Response({ - "show_notifications_tray": show_notifications_tray, - "count": count_total, - "count_by_app_name": count_by_app_name_dict, - "notification_expiry_days": settings.NOTIFICATIONS_EXPIRY, - }) + logger.info( + 'Retrieved notification count for user %s: total=%d', + request.user.id, + count_total + ) + return Response({ + "show_notifications_tray": show_notifications_tray, + "count": count_total, + "count_by_app_name": count_by_app_name_dict, + "notification_expiry_days": settings.NOTIFICATIONS_EXPIRY, + }) + except (Notification.DoesNotExist, AttributeError) as exc: + logger.error( + 'Failed to retrieve notification count for user %s: %s', + request.user.id, + str(exc), + ) + return Response( + {'error': 'Failed to retrieve notification count'}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) @allow_any_authenticated_user() class MarkNotificationsSeenAPIView(UpdateAPIView): """ - API view for marking user's all notifications seen for a provided app_name. + API view for marking user notifications seen for app_name. """ def update(self, request, *args, **kwargs): """ - Marks all notifications for the given app name seen for the authenticated user. + Mark all notifications for app name as seen. **Args:** app_name: The name of the app to mark notifications seen for. **Response Format:** - A `Response` object with a 200 OK status code if the notifications were successfully marked seen. + A `Response` object with 200 OK if notifications marked seen. **Response Error Codes**: - - 400: Bad Request status code if the app name is invalid. + - 400: Bad Request if app name is invalid. """ - app_name = self.kwargs.get('app_name') - - if not app_name: - return Response({'error': _('Invalid app name.')}, status=400) - - notifications = Notification.objects.filter( - user=request.user, - app_name=app_name, - last_seen__isnull=True, - ) - - notifications.update(last_seen=datetime.now()) + try: + app_name = self.kwargs.get('app_name') + + if not app_name: + logger.warning( + f'Invalid app_name provided by user {request.user.id}' + ) + return Response( + {'error': _('Invalid app name.')}, + status=status.HTTP_400_BAD_REQUEST + ) - return Response({'message': _('Notifications marked as seen.')}, status=200) + notifications = Notification.objects.filter( + user=request.user, + app_name=app_name, + last_seen__isnull=True, + ) + update_count = notifications.update(last_seen=datetime.now(UTC)) + logger.info( + 'Marked %d notifications as seen for user %s with app_name=%s', + update_count, + request.user.id, + app_name, + ) + return Response( + {'message': _('Notifications marked as seen.')}, + status=status.HTTP_200_OK, + ) + except (Notification.DoesNotExist, AttributeError, TypeError) as exc: + logger.error( + 'Failed to mark notifications seen for user %s: %s', + request.user.id, + str(exc), + ) + return Response( + {'error': _('Failed to mark notifications as seen.')}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) @allow_any_authenticated_user() class NotificationReadAPIView(APIView): """ - API view for marking user notifications as read, either all notifications or a single notification + API view for marking notifications as read. """ def patch(self, request, *args, **kwargs): """ - Marks all notifications or single notification read for the given - app name or notification id for the authenticated user. + Mark all or single notification as read. Requests: PATCH /api/notifications/read/ Parameters: - request (Request): The request object containing the app name or notification id. + request (Request): Request containing app name or notification id. { "app_name": (str) app_name, "notification_id": (int) notification_id } Returns: - - 200: OK status code if the notification or notifications were successfully marked read. - - 400: Bad Request status code if the app name is invalid. - - 403: Forbidden status code if the user is not authenticated. - - 404: Not Found status code if the notification was not found. + - 200: OK if notification marked read. + - 400: Bad Request if app name or notification id is invalid. + - 403: Forbidden if user not authenticated. + - 404: Not Found if notification not found. """ - notification_id = request.data.get('notification_id', None) - read_at = datetime.now(UTC) - - if notification_id: - notification = get_object_or_404(Notification, pk=notification_id, user=request.user) - first_time_read = notification.last_read is None - notification.last_read = read_at - notification.save() - notification_read_event(request.user, notification, first_time_read) - return Response({'message': _('Notification marked read.')}, status=status.HTTP_200_OK) + notification_id = request.data.get('notification_id') + app_name = request.data.get('app_name') + + # Require at least one identifier. + if not notification_id and not app_name: + logger.warning( + 'Invalid app_name (%s) or notification_id (%s) from user %s', + app_name, + notification_id, + request.user.id, + ) + return Response( + {'error': _('Invalid app_name or notification_id.')}, + status=status.HTTP_400_BAD_REQUEST, + ) - app_name = request.data.get('app_name', '') + read_at = datetime.now(UTC) - if app_name in COURSE_NOTIFICATION_APPS: - notifications = Notification.objects.filter( - user=request.user, - app_name=app_name, - last_read__isnull=True, + try: + # If notification_id is provided, it takes precedence + # over app_name. + if notification_id: + notification = get_object_or_404( + Notification, + pk=notification_id, + user=request.user, + ) + first_time_read = notification.last_read is None + notification.last_read = read_at + notification.save() + notification_read_event( + request.user, + notification, + first_time_read, + ) + logger.info( + 'Marked notification %s as read for user %s', + notification_id, + request.user.id, + ) + return Response( + {'message': _('Notification marked read.')}, + status=status.HTTP_200_OK, + ) + + # If app_name is provided, mark all unread notifications for that app. + if app_name in COURSE_NOTIFICATION_APPS: + notifications = Notification.objects.filter( + user=request.user, + app_name=app_name, + last_read__isnull=True, + ) + update_count = notifications.update(last_read=read_at) + notifications_app_all_read_event(request.user, app_name) + logger.info( + 'Marked %d notifications as read for user %s with app_name=%s', + update_count, + request.user.id, + app_name, + ) + return Response( + {'message': _('Notifications marked read.')}, + status=status.HTTP_200_OK, + ) + + logger.warning( + 'Invalid app_name (%s) or notification_id (%s) from user %s', + app_name, + notification_id, + request.user.id, + ) + return Response( + {'error': _('Invalid app_name or notification_id.')}, + status=status.HTTP_400_BAD_REQUEST, + ) + except (Notification.DoesNotExist, AttributeError, TypeError) as exc: + logger.error( + 'Failed to mark notification as read for user %s: %s', + request.user.id, + str(exc), + ) + return Response( + {'error': _('Failed to mark notification as read.')}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) - notifications.update(last_read=read_at) - notifications_app_all_read_event(request.user, app_name) - return Response({'message': _('Notifications marked read.')}, status=status.HTTP_200_OK) - return Response({'error': _('Invalid app_name or notification_id.')}, status=status.HTTP_400_BAD_REQUEST) +@api_view(["GET", "POST"]) +def preference_update_from_encrypted_username_view(request, username, patch=""): # pylint: disable=unused-argument + """Update notification preferences from an encrypted username. -@api_view(['GET', 'POST']) -def preference_update_from_encrypted_username_view(request, username, patch=""): - """ - View to update user preferences from encrypted username and patch. - username and patch must be string + Used by one-click unsubscribe links. Rate limited using + ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT. """ if is_ratelimited( - request=request, group="unsubscribe", key=username_from_hash, - rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, increment=True, + request=request, + group="unsubscribe", + key=username_from_hash, + rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, + increment=True, ): - return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS) - update_user_preferences_from_patch(username) - return Response({"result": "success"}, status=status.HTTP_200_OK) + logger.warning( + "Rate limit exceeded for username hash: %s", + username, + ) + return Response( + {"error": "Too many requests"}, + status=status.HTTP_429_TOO_MANY_REQUESTS, + ) + + try: + update_user_preferences_from_patch(username) + logger.info( + "Updated preferences for encrypted username: %s", + username, + ) + return Response({"result": "success"}, status=status.HTTP_200_OK) + except ValueError as exc: + logger.error( + "Failed to update preferences for encrypted username %s: %s", + username, + str(exc), + ) + return Response( + {"error": "Failed to update preferences"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) @allow_any_authenticated_user() class NotificationPreferencesView(APIView): """ - API view to retrieve and structure the notification preferences for the - authenticated user. + API view to retrieve and structure notification preferences. """ def get(self, request): """ - Handles GET requests to retrieve notification preferences. - - This method fetches the user's active notification preferences and - merges them with a default structure provided by NotificationAppManager. - This provides a complete view of all possible notifications and the - user's current settings for them. + Retrieve notification preferences for authenticated user. Returns: - Response: A DRF Response object containing the structured - notification preferences or an error message. + Response: DRF Response with structured notification preferences. """ - user_preferences_qs = NotificationPreference.objects.filter(user=request.user) - user_preferences_map = {pref.type: pref for pref in user_preferences_qs} - - # Ensure all notification types are present in the user's preferences. - # If any are missing, create them with default values. - diff = set(COURSE_NOTIFICATION_TYPES.keys()) - set(user_preferences_map.keys()) - missing_types = [] - for missing_type in diff: - new_pref = create_notification_preference( - user_id=request.user.id, - notification_type=missing_type, - - ) - missing_types.append(new_pref) - user_preferences_map[missing_type] = new_pref - if missing_types: - NotificationPreference.objects.bulk_create(missing_types) - - # If no user preferences are found, return an error response. - if not user_preferences_map: + try: + user_preferences_qs = NotificationPreference.objects.filter( + user=request.user + ) + user_preferences_map = { + pref.type: pref for pref in user_preferences_qs + } + + # Ensure all notification types present in user's preferences. + diff = set(COURSE_NOTIFICATION_TYPES.keys()) - set( + user_preferences_map.keys() + ) + missing_types = [] + for missing_type in diff: + new_pref = create_notification_preference( + user_id=request.user.id, + notification_type=missing_type, + ) + missing_types.append(new_pref) + user_preferences_map[missing_type] = new_pref + if missing_types: + NotificationPreference.objects.bulk_create(missing_types) + logger.info( + 'Created %d missing notification preferences for user %s', + len(missing_types), + request.user.id + ) + + # If no user preferences found, return error response. + if not user_preferences_map: + logger.warning( + 'No active notification preferences for user %s', + request.user.id + ) + return Response({ + 'status': 'error', + 'message': 'No active notification preferences found.' + }, status=status.HTTP_404_NOT_FOUND) + + # Get structured preferences from NotificationAppManager. + structured_preferences = ( + NotificationAppManager() + .get_notification_app_preferences() + ) + + for app_name, app_settings in structured_preferences.items(): + notification_types = app_settings.get( + 'notification_types', {} + ) + + # Process notification types. + for type_name, type_details in notification_types.items(): + if type_name == 'core': + if structured_preferences[app_name][ + 'core_notification_types' + ]: + notification_type = ( + structured_preferences[app_name] + ['core_notification_types'][0] + ) + else: + notification_type = 'core' + user_pref = user_preferences_map.get( + notification_type + ) + else: + user_pref = user_preferences_map.get(type_name) + if user_pref: + # Update dictionary for this type. + type_details['web'] = user_pref.web + 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 + ) + structured_preferences = add_non_editable_in_preference( + add_info_to_notification_config(structured_preferences) + ) + logger.info( + 'Retrieved notification preferences for user %s', + request.user.id, + ) + return Response({ + 'status': 'success', + 'message': 'Notification preferences retrieved successfully.', + 'show_preferences': get_show_notifications_tray( + self.request.user + ), + 'data': structured_preferences + }, status=status.HTTP_200_OK) + except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: + logger.error( + 'Failed to retrieve notification preferences for user %s: %s', + request.user.id, + str(exc), + ) return Response({ 'status': 'error', - 'message': 'No active notification preferences found for this user.' - }, status=status.HTTP_404_NOT_FOUND) - - # Get the structured preferences from the NotificationAppManager. - # This will include all apps and their notification types. - structured_preferences = NotificationAppManager().get_notification_app_preferences() - - for app_name, app_settings in structured_preferences.items(): - notification_types = app_settings.get('notification_types', {}) - - # Process all notification types (core and non-core) in a single loop. - for type_name, type_details in notification_types.items(): - if type_name == 'core': - if structured_preferences[app_name]['core_notification_types']: - # If the app has core notification types, use the first one as the type name. - # This assumes that the first core notification type is representative of the core settings. - notification_type = structured_preferences[app_name]['core_notification_types'][0] - else: - notification_type = 'core' - user_pref = user_preferences_map.get(notification_type) - else: - user_pref = user_preferences_map.get(type_name) - if user_pref: - # If a preference exists, update the dictionary for this type. - # This directly modifies the 'type_details' dictionary. - type_details['web'] = user_pref.web - 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) - structured_preferences = add_non_editable_in_preference( - add_info_to_notification_config(structured_preferences) - ) - return Response({ - 'status': 'success', - 'message': 'Notification preferences retrieved successfully.', - 'show_preferences': get_show_notifications_tray(self.request.user), - 'data': structured_preferences - }, status=status.HTTP_200_OK) + 'message': 'Failed to retrieve notification preferences.' + }, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def put(self, request): """ - Handles PUT requests to update notification preferences. - - This method updates the user's notification preferences based on the - provided data in the request body. It expects a dictionary with - notification types and their settings. + Update notification preferences for authenticated user. Returns: - Response: A DRF Response object indicating success or failure. + Response: DRF Response indicating success or failure. """ - # Validate incoming data - serializer = UserNotificationPreferenceUpdateAllSerializer(data=request.data) - if not serializer.is_valid(): - return Response({ - 'status': 'error', - 'message': serializer.errors - }, status=status.HTTP_400_BAD_REQUEST) - - # Get validated data for easier access - validated_data = serializer.validated_data - - # Build query set based on notification type - query_set = NotificationPreference.objects.filter(user_id=request.user.id) - - if validated_data['notification_type'] == 'core': - # Get core notification types for the app - __, core_types = NotificationTypeManager().get_notification_app_preference( - notification_app=validated_data['notification_app'] + try: + # Validate incoming data + serializer = UserNotificationPreferenceUpdateAllSerializer( + data=request.data + ) + if not serializer.is_valid(): + logger.warning( + 'Invalid serializer data for user %s: %s', + request.user.id, + serializer.errors + ) + return Response({ + 'status': 'error', + 'message': serializer.errors + }, status=status.HTTP_400_BAD_REQUEST) + + # Get validated data + validated_data = serializer.validated_data + + # Build query set based on notification type + query_set = NotificationPreference.objects.filter( + user_id=request.user.id ) - query_set = query_set.filter(type__in=core_types) - else: - # Filter by single notification type - query_set = query_set.filter(type=validated_data['notification_type']) - - # Prepare update data based on channel type - updated_data = self._prepare_update_data(validated_data) - - # Update preferences - query_set.update(**updated_data) - - # Log the event - self._log_preference_update_event(request.user, validated_data) - # Prepare and return response - response_data = self._prepare_response_data(validated_data) - return Response(response_data, status=status.HTTP_200_OK) + if validated_data['notification_type'] == 'core': + # Get core notification types for the app + __, core_types = NotificationTypeManager( + ).get_notification_app_preference( + notification_app=validated_data['notification_app'] + ) + query_set = query_set.filter(type__in=core_types) + else: + # Filter by single notification type + query_set = query_set.filter( + type=validated_data['notification_type'] + ) + + # Prepare update data + updated_data = self._prepare_update_data(validated_data) + + # Update preferences + update_count = query_set.update(**updated_data) + + # Log the event + self._log_preference_update_event(request.user, validated_data) + logger.info( + 'Updated %d notification preferences for user %s with app=%s', + update_count, + request.user.id, + validated_data["notification_app"], + ) + # Prepare and return response + response_data = self._prepare_response_data(validated_data) + return Response(response_data, status=status.HTTP_200_OK) + except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: + logger.error( + 'Failed to update notification preferences for user %s: %s', + request.user.id, + str(exc), + ) + return Response({ + 'status': 'error', + 'message': 'Failed to update notification preferences.' + }, status=status.HTTP_500_INTERNAL_SERVER_ERROR) def _prepare_update_data(self, validated_data): """ - Prepare the data dictionary for updating notification preferences. + Prepare the data dictionary for updating preferences. Args: validated_data (dict): Validated serializer data @@ -392,12 +621,22 @@ def _prepare_update_data(self, validated_data): Returns: dict: Dictionary with update data """ - channel = validated_data['notification_channel'] - - if channel == 'email_cadence': - return {channel: validated_data['email_cadence']} - else: - return {channel: validated_data['value']} + try: + channel = validated_data['notification_channel'] + + if channel == 'email_cadence': + result = {channel: validated_data['email_cadence']} + else: + result = {channel: validated_data['value']} + logger.debug( + 'Prepared update data for channel %s: %s', + channel, + result, + ) + return result + except KeyError as exc: + logger.error('Failed to prepare update data: %s', str(exc)) + raise def _log_preference_update_event(self, user, validated_data): """ @@ -407,14 +646,27 @@ def _log_preference_update_event(self, user, validated_data): user: The user making the update validated_data (dict): Validated serializer data """ - event_data = { - 'notification_app': validated_data['notification_app'], - 'notification_type': validated_data['notification_type'], - 'notification_channel': validated_data['notification_channel'], - 'value': validated_data.get('value'), - 'email_cadence': validated_data.get('email_cadence'), - } - notification_preference_update_event(user, [], event_data) + try: + event_data = { + 'notification_app': validated_data['notification_app'], + 'notification_type': validated_data['notification_type'], + 'notification_channel': validated_data[ + 'notification_channel' + ], + 'value': validated_data.get('value'), + 'email_cadence': validated_data.get('email_cadence'), + } + notification_preference_update_event(user, [], event_data) + logger.debug( + 'Logged preference update events for user %s', + user.id + ) + except (KeyError, AttributeError, TypeError, ValueError) as exc: + logger.error( + 'Failed to log preference update events for user %s: %s', + user.id, + str(exc) + ) def _prepare_response_data(self, validated_data): """ @@ -428,7 +680,10 @@ def _prepare_response_data(self, validated_data): """ email_cadence = validated_data.get('email_cadence', None) # Determine the updated value - updated_value = validated_data.get('value', email_cadence if email_cadence else None) + updated_value = validated_data.get( + 'value', + email_cadence if email_cadence else None + ) # Determine the channel channel = validated_data.get('notification_channel') @@ -438,7 +693,9 @@ def _prepare_response_data(self, validated_data): return { 'status': 'success', 'message': 'Notification preferences update completed', - 'show_preferences': get_show_notifications_tray(self.request.user), + 'show_preferences': get_show_notifications_tray( + self.request.user + ), 'data': { 'updated_value': updated_value, 'notification_type': validated_data['notification_type'], @@ -446,3 +703,4 @@ def _prepare_response_data(self, validated_data): 'app': validated_data['notification_app'], } } + \ No newline at end of file From 2410870aa097b1b2da051e7f3385f2c1ec23ab59 Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Mon, 2 Mar 2026 11:58:34 +0000 Subject: [PATCH 2/6] feat: added extra line at the end of the file since it required to pass the quality check pipeline --- openedx/core/djangoapps/notifications/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 8f4991117dd9..8329be4ee611 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -703,4 +703,3 @@ def _prepare_response_data(self, validated_data): 'app': validated_data['notification_app'], } } - \ No newline at end of file From 447671c3af9e0ccffd5154a75d0abca43dc5137a Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Tue, 3 Mar 2026 11:38:44 +0000 Subject: [PATCH 3/6] chore: resolved copilot suggesstions --- .../core/djangoapps/notifications/views.py | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 8329be4ee611..d147c6da38dc 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -5,7 +5,9 @@ from datetime import datetime, timedelta from django.conf import settings +from django.core.exceptions import BadRequest from django.db.models import Count +from django.http import Http404 from django_ratelimit.core import is_ratelimited from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ @@ -16,6 +18,7 @@ from rest_framework.response import Response from rest_framework.views import APIView +from lms.djangoapps.discussion.notification_prefs.views import UsernameDecryptionException from openedx.core.djangoapps.notifications.email.utils import ( update_user_preferences_from_patch, username_from_hash @@ -394,8 +397,7 @@ def preference_update_from_encrypted_username_view(request, username, patch=""): increment=True, ): logger.warning( - "Rate limit exceeded for username hash: %s", - username, + "Rate limit exceeded for one-click unsubscribe request" ) return Response( {"error": "Too many requests"}, @@ -405,19 +407,33 @@ def preference_update_from_encrypted_username_view(request, username, patch=""): try: update_user_preferences_from_patch(username) logger.info( - "Updated preferences for encrypted username: %s", - username, + "Updated preferences from one-click unsubscribe request" ) return Response({"result": "success"}, status=status.HTTP_200_OK) - except ValueError as exc: - logger.error( - "Failed to update preferences for encrypted username %s: %s", - username, + except UsernameDecryptionException: + logger.warning( + "Invalid encrypted username token in one-click unsubscribe request" + ) + return Response( + {"error": "Invalid token"}, + status=status.HTTP_400_BAD_REQUEST, + ) + except Http404: + logger.warning( + "User not found for one-click unsubscribe request" + ) + return Response( + {"error": "User not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + except BadRequest as exc: + logger.warning( + "Bad request in one-click unsubscribe: %s", str(exc), ) return Response( - {"error": "Failed to update preferences"}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, + {"error": "Bad request"}, + status=status.HTTP_400_BAD_REQUEST, ) From 4ead051d5a55ee688c0f14228a4c411bd6328d70 Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Tue, 3 Mar 2026 12:01:30 +0000 Subject: [PATCH 4/6] chore: improve error handling loggers --- openedx/core/djangoapps/notifications/views.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index d147c6da38dc..fd1e5f692e3f 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -200,10 +200,10 @@ def get(self, request): "count_by_app_name": count_by_app_name_dict, "notification_expiry_days": settings.NOTIFICATIONS_EXPIRY, }) - except (Notification.DoesNotExist, AttributeError) as exc: + except (AttributeError, TypeError) as exc: logger.error( 'Failed to retrieve notification count for user %s: %s', - request.user.id, + getattr(request.user, 'id', None), str(exc), ) return Response( @@ -260,7 +260,7 @@ def update(self, request, *args, **kwargs): except (Notification.DoesNotExist, AttributeError, TypeError) as exc: logger.error( 'Failed to mark notifications seen for user %s: %s', - request.user.id, + getattr(request.user, 'id', None), str(exc), ) return Response( @@ -370,10 +370,10 @@ def patch(self, request, *args, **kwargs): {'error': _('Invalid app_name or notification_id.')}, status=status.HTTP_400_BAD_REQUEST, ) - except (Notification.DoesNotExist, AttributeError, TypeError) as exc: + except (AttributeError, TypeError) as exc: logger.error( 'Failed to mark notification as read for user %s: %s', - request.user.id, + getattr(request.user, 'id', None), str(exc), ) return Response( @@ -547,7 +547,7 @@ def get(self, request): except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: logger.error( 'Failed to retrieve notification preferences for user %s: %s', - request.user.id, + getattr(request.user, 'id', None), str(exc), ) return Response({ @@ -619,7 +619,7 @@ def put(self, request): except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: logger.error( 'Failed to update notification preferences for user %s: %s', - request.user.id, + getattr(request.user, 'id', None), str(exc), ) return Response({ From 56af9af2017ac1c6192438317a6a8854a0813f87 Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Tue, 3 Mar 2026 12:19:34 +0000 Subject: [PATCH 5/6] chore: improve Notification error handling loggers --- .../core/djangoapps/notifications/views.py | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index fd1e5f692e3f..75e7f9ca9486 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -389,13 +389,24 @@ def preference_update_from_encrypted_username_view(request, username, patch=""): Used by one-click unsubscribe links. Rate limited using ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT. """ - if is_ratelimited( - request=request, - group="unsubscribe", - key=username_from_hash, - rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, - increment=True, - ): + try: + is_rate_limited = is_ratelimited( + request=request, + group="unsubscribe", + key=username_from_hash, + rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, + increment=True, + ) + except BadRequest: + logger.warning( + "Invalid encrypted username token in one-click unsubscribe request" + ) + return Response( + {"error": "Invalid token"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + if is_rate_limited: logger.warning( "Rate limit exceeded for one-click unsubscribe request" ) From e68097f85e1d6738f06031cca83a9c0242f4ba1e Mon Sep 17 00:00:00 2001 From: Ram Mohan Vamguru Date: Fri, 13 Mar 2026 12:39:15 +0000 Subject: [PATCH 6/6] feat: Add logging statements to Notifications API - Added logger.info() at API entry points - Added logger.warning() for invalid inputs - Log update counts for meaningful operations - No logic or formatting changes - Minimal PR diff --- .../core/djangoapps/notifications/views.py | 777 +++++++----------- 1 file changed, 285 insertions(+), 492 deletions(-) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 75e7f9ca9486..ef7c9a78686e 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -5,9 +5,7 @@ from datetime import datetime, timedelta from django.conf import settings -from django.core.exceptions import BadRequest from django.db.models import Count -from django.http import Http404 from django_ratelimit.core import is_ratelimited from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as _ @@ -18,22 +16,12 @@ from rest_framework.response import Response from rest_framework.views import APIView -from lms.djangoapps.discussion.notification_prefs.views import UsernameDecryptionException -from openedx.core.djangoapps.notifications.email.utils import ( - update_user_preferences_from_patch, - username_from_hash -) +from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch, username_from_hash from openedx.core.djangoapps.notifications.models import NotificationPreference -from openedx.core.djangoapps.notifications.permissions import ( - allow_any_authenticated_user -) +from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user -from .base_notification import ( - COURSE_NOTIFICATION_APPS, - NotificationAppManager, - COURSE_NOTIFICATION_TYPES, +from .base_notification import COURSE_NOTIFICATION_APPS, NotificationAppManager, COURSE_NOTIFICATION_TYPES, \ NotificationTypeManager -) from .events import ( notification_preference_update_event, notification_read_event, @@ -93,58 +81,40 @@ class NotificationListAPIView(generics.ListAPIView): def get_queryset(self): """ - Override get_queryset to filter by app name, user and created. + Override the get_queryset method to filter the queryset by app name, request.user and created """ - try: - expiry_date = datetime.now(UTC) - timedelta( - days=settings.NOTIFICATIONS_EXPIRY - ) - app_name = self.request.query_params.get('app_name') - - if self.request.query_params.get('tray_opened'): - unseen_count = Notification.objects.filter( - user_id=self.request.user, - last_seen__isnull=True - ).count() - notification_tray_opened_event( - self.request.user, - unseen_count - ) - params = { - 'user': self.request.user, - 'created__gte': expiry_date, - 'web': True - } + expiry_date = datetime.now(UTC) - timedelta(days=settings.NOTIFICATIONS_EXPIRY) + app_name = self.request.query_params.get('app_name') + + if self.request.query_params.get('tray_opened'): + unseen_count = Notification.objects.filter(user_id=self.request.user, last_seen__isnull=True).count() + notification_tray_opened_event(self.request.user, unseen_count) + params = { + 'user': self.request.user, + 'created__gte': expiry_date, + 'web': True + } - if app_name: - params['app_name'] = app_name - queryset = Notification.objects.filter( - **params - ).order_by('-created') - logger.info( - 'Retrieved notifications for user %s with app_name=%s', - self.request.user.id, - app_name, - ) - return queryset - except Notification.DoesNotExist as exc: - logger.error( - 'Failed to retrieve notifications for user %s: %s', - self.request.user.id, - str(exc) - ) - raise + if app_name: + params['app_name'] = app_name + queryset = Notification.objects.filter(**params).order_by('-created') + logger.debug( + 'Built notifications queryset for user %s with app_name=%s', + self.request.user.id, + app_name, + ) + return queryset @allow_any_authenticated_user() class NotificationCountView(APIView): """ - API view for getting unseen notifications count and tray flag. + API view for getting the unseen notifications count and show_notification_tray flag for a user. """ def get(self, request): """ - Get the unseen notifications count and show_notification_tray flag. + Get the unseen notifications count and show_notification_tray flag for a user. **Permissions**: User must be authenticated. **Response Format**: @@ -162,485 +132,336 @@ def get(self, request): **Response Error Codes**: - 403: The requester cannot access resource. """ - try: - # Get unseen notifications count for each app name. - count_by_app_name = ( - Notification.objects - .filter( - user_id=request.user, - last_seen__isnull=True, - web=True - ) - .values('app_name') - .annotate(count=Count('*')) - ) - count_total = 0 - show_notifications_tray = get_show_notifications_tray( - self.request.user - ) - count_by_app_name_dict = { - app_name: 0 - for app_name in COURSE_NOTIFICATION_APPS - } + # Get the unseen notifications count for each app name. + count_by_app_name = ( + Notification.objects + .filter(user_id=request.user, last_seen__isnull=True, web=True) + .values('app_name') + .annotate(count=Count('*')) + ) + count_total = 0 + show_notifications_tray = get_show_notifications_tray(self.request.user) + count_by_app_name_dict = { + app_name: 0 + for app_name in COURSE_NOTIFICATION_APPS + } - for item in count_by_app_name: - app_name = item['app_name'] - count = item['count'] - count_total += count - count_by_app_name_dict[app_name] = count + for item in count_by_app_name: + app_name = item['app_name'] + count = item['count'] + count_total += count + count_by_app_name_dict[app_name] = count - logger.info( - 'Retrieved notification count for user %s: total=%d', - request.user.id, - count_total - ) - return Response({ - "show_notifications_tray": show_notifications_tray, - "count": count_total, - "count_by_app_name": count_by_app_name_dict, - "notification_expiry_days": settings.NOTIFICATIONS_EXPIRY, - }) - except (AttributeError, TypeError) as exc: - logger.error( - 'Failed to retrieve notification count for user %s: %s', - getattr(request.user, 'id', None), - str(exc), - ) - return Response( - {'error': 'Failed to retrieve notification count'}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, - ) + logger.debug( + 'Retrieved notification count for user %s: total=%d', + request.user.id, + count_total + ) + return Response({ + "show_notifications_tray": show_notifications_tray, + "count": count_total, + "count_by_app_name": count_by_app_name_dict, + "notification_expiry_days": settings.NOTIFICATIONS_EXPIRY, + }) @allow_any_authenticated_user() class MarkNotificationsSeenAPIView(UpdateAPIView): """ - API view for marking user notifications seen for app_name. + API view for marking user's all notifications seen for a provided app_name. """ def update(self, request, *args, **kwargs): """ - Mark all notifications for app name as seen. + Marks all notifications for the given app name seen for the authenticated user. **Args:** app_name: The name of the app to mark notifications seen for. **Response Format:** - A `Response` object with 200 OK if notifications marked seen. + A `Response` object with a 200 OK status code if the notifications were successfully marked seen. **Response Error Codes**: - - 400: Bad Request if app name is invalid. + - 400: Bad Request status code if the app name is invalid. """ - try: - app_name = self.kwargs.get('app_name') - - if not app_name: - logger.warning( - f'Invalid app_name provided by user {request.user.id}' - ) - return Response( - {'error': _('Invalid app name.')}, - status=status.HTTP_400_BAD_REQUEST - ) + app_name = self.kwargs.get('app_name') - notifications = Notification.objects.filter( - user=request.user, - app_name=app_name, - last_seen__isnull=True, - ) - update_count = notifications.update(last_seen=datetime.now(UTC)) - logger.info( - 'Marked %d notifications as seen for user %s with app_name=%s', - update_count, - request.user.id, - app_name, - ) - return Response( - {'message': _('Notifications marked as seen.')}, - status=status.HTTP_200_OK, - ) - except (Notification.DoesNotExist, AttributeError, TypeError) as exc: - logger.error( - 'Failed to mark notifications seen for user %s: %s', - getattr(request.user, 'id', None), - str(exc), - ) - return Response( - {'error': _('Failed to mark notifications as seen.')}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, + if not app_name: + logger.warning( + 'Invalid app_name provided by user %s', + request.user.id ) + return Response({'error': _('Invalid app name.')}, status=400) + + notifications = Notification.objects.filter( + user=request.user, + app_name=app_name, + last_seen__isnull=True, + ) + + update_count = notifications.update(last_seen=datetime.now(UTC)) + logger.info( + 'Marked %d notifications as seen for user %s with app_name=%s', + update_count, + request.user.id, + app_name, + ) + + return Response({'message': _('Notifications marked as seen.')}, status=200) @allow_any_authenticated_user() class NotificationReadAPIView(APIView): """ - API view for marking notifications as read. + API view for marking user notifications as read, either all notifications or a single notification """ def patch(self, request, *args, **kwargs): """ - Mark all or single notification as read. + Marks all notifications or single notification read for the given + app name or notification id for the authenticated user. Requests: PATCH /api/notifications/read/ Parameters: - request (Request): Request containing app name or notification id. + request (Request): The request object containing the app name or notification id. { "app_name": (str) app_name, "notification_id": (int) notification_id } Returns: - - 200: OK if notification marked read. - - 400: Bad Request if app name or notification id is invalid. - - 403: Forbidden if user not authenticated. - - 404: Not Found if notification not found. + - 200: OK status code if the notification or notifications were successfully marked read. + - 400: Bad Request status code if the app name is invalid. + - 403: Forbidden status code if the user is not authenticated. + - 404: Not Found status code if the notification was not found. """ - notification_id = request.data.get('notification_id') - app_name = request.data.get('app_name') + notification_id = request.data.get('notification_id', None) + read_at = datetime.now(UTC) - # Require at least one identifier. - if not notification_id and not app_name: - logger.warning( - 'Invalid app_name (%s) or notification_id (%s) from user %s', - app_name, + if notification_id: + notification = get_object_or_404(Notification, pk=notification_id, user=request.user) + first_time_read = notification.last_read is None + notification.last_read = read_at + notification.save() + notification_read_event(request.user, notification, first_time_read) + logger.info( + 'Marked notification %s as read for user %s', notification_id, request.user.id, ) - return Response( - {'error': _('Invalid app_name or notification_id.')}, - status=status.HTTP_400_BAD_REQUEST, - ) - - read_at = datetime.now(UTC) + return Response({'message': _('Notification marked read.')}, status=status.HTTP_200_OK) - try: - # If notification_id is provided, it takes precedence - # over app_name. - if notification_id: - notification = get_object_or_404( - Notification, - pk=notification_id, - user=request.user, - ) - first_time_read = notification.last_read is None - notification.last_read = read_at - notification.save() - notification_read_event( - request.user, - notification, - first_time_read, - ) - logger.info( - 'Marked notification %s as read for user %s', - notification_id, - request.user.id, - ) - return Response( - {'message': _('Notification marked read.')}, - status=status.HTTP_200_OK, - ) - - # If app_name is provided, mark all unread notifications for that app. - if app_name in COURSE_NOTIFICATION_APPS: - notifications = Notification.objects.filter( - user=request.user, - app_name=app_name, - last_read__isnull=True, - ) - update_count = notifications.update(last_read=read_at) - notifications_app_all_read_event(request.user, app_name) - logger.info( - 'Marked %d notifications as read for user %s with app_name=%s', - update_count, - request.user.id, - app_name, - ) - return Response( - {'message': _('Notifications marked read.')}, - status=status.HTTP_200_OK, - ) + app_name = request.data.get('app_name', '') - logger.warning( - 'Invalid app_name (%s) or notification_id (%s) from user %s', - app_name, - notification_id, - request.user.id, - ) - return Response( - {'error': _('Invalid app_name or notification_id.')}, - status=status.HTTP_400_BAD_REQUEST, - ) - except (AttributeError, TypeError) as exc: - logger.error( - 'Failed to mark notification as read for user %s: %s', - getattr(request.user, 'id', None), - str(exc), + if app_name in COURSE_NOTIFICATION_APPS: + notifications = Notification.objects.filter( + user=request.user, + app_name=app_name, + last_read__isnull=True, ) - return Response( - {'error': _('Failed to mark notification as read.')}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, + update_count = notifications.update(last_read=read_at) + notifications_app_all_read_event(request.user, app_name) + logger.info( + 'Marked %d notifications as read for user %s with app_name=%s', + update_count, + request.user.id, + app_name, ) + return Response({'message': _('Notifications marked read.')}, status=status.HTTP_200_OK) - -@api_view(["GET", "POST"]) -def preference_update_from_encrypted_username_view(request, username, patch=""): # pylint: disable=unused-argument - """Update notification preferences from an encrypted username. - - Used by one-click unsubscribe links. Rate limited using - ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT. - """ - try: - is_rate_limited = is_ratelimited( - request=request, - group="unsubscribe", - key=username_from_hash, - rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, - increment=True, - ) - except BadRequest: logger.warning( - "Invalid encrypted username token in one-click unsubscribe request" - ) - return Response( - {"error": "Invalid token"}, - status=status.HTTP_400_BAD_REQUEST, + 'Invalid app_name (%s) or notification_id (%s) from user %s', + app_name, + notification_id, + request.user.id, ) + return Response({'error': _('Invalid app_name or notification_id.')}, status=status.HTTP_400_BAD_REQUEST) - if is_rate_limited: - logger.warning( - "Rate limit exceeded for one-click unsubscribe request" - ) - return Response( - {"error": "Too many requests"}, - status=status.HTTP_429_TOO_MANY_REQUESTS, - ) - try: - update_user_preferences_from_patch(username) - logger.info( - "Updated preferences from one-click unsubscribe request" - ) - return Response({"result": "success"}, status=status.HTTP_200_OK) - except UsernameDecryptionException: - logger.warning( - "Invalid encrypted username token in one-click unsubscribe request" - ) - return Response( - {"error": "Invalid token"}, - status=status.HTTP_400_BAD_REQUEST, - ) - except Http404: - logger.warning( - "User not found for one-click unsubscribe request" - ) - return Response( - {"error": "User not found"}, - status=status.HTTP_404_NOT_FOUND, - ) - except BadRequest as exc: +@api_view(['GET', 'POST']) +def preference_update_from_encrypted_username_view(request, username, patch=""): + """ + View to update user preferences from encrypted username and patch. + username and patch must be string + """ + if is_ratelimited( + request=request, group="unsubscribe", key=username_from_hash, + rate=settings.ONE_CLICK_UNSUBSCRIBE_RATE_LIMIT, increment=True, + ): logger.warning( - "Bad request in one-click unsubscribe: %s", - str(exc), - ) - return Response( - {"error": "Bad request"}, - status=status.HTTP_400_BAD_REQUEST, + 'Rate limit exceeded for one-click unsubscribe request' ) + return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS) + update_user_preferences_from_patch(username) + logger.info( + 'Updated preferences from one-click unsubscribe request' + ) + return Response({"result": "success"}, status=status.HTTP_200_OK) @allow_any_authenticated_user() class NotificationPreferencesView(APIView): """ - API view to retrieve and structure notification preferences. + API view to retrieve and structure the notification preferences for the + authenticated user. """ def get(self, request): """ - Retrieve notification preferences for authenticated user. + Handles GET requests to retrieve notification preferences. + + This method fetches the user's active notification preferences and + merges them with a default structure provided by NotificationAppManager. + This provides a complete view of all possible notifications and the + user's current settings for them. Returns: - Response: DRF Response with structured notification preferences. + Response: A DRF Response object containing the structured + notification preferences or an error message. """ - try: - user_preferences_qs = NotificationPreference.objects.filter( - user=request.user - ) - user_preferences_map = { - pref.type: pref for pref in user_preferences_qs - } - - # Ensure all notification types present in user's preferences. - diff = set(COURSE_NOTIFICATION_TYPES.keys()) - set( - user_preferences_map.keys() - ) - missing_types = [] - for missing_type in diff: - new_pref = create_notification_preference( - user_id=request.user.id, - notification_type=missing_type, - ) - missing_types.append(new_pref) - user_preferences_map[missing_type] = new_pref - if missing_types: - NotificationPreference.objects.bulk_create(missing_types) - logger.info( - 'Created %d missing notification preferences for user %s', - len(missing_types), - request.user.id - ) - - # If no user preferences found, return error response. - if not user_preferences_map: - logger.warning( - 'No active notification preferences for user %s', - request.user.id - ) - return Response({ - 'status': 'error', - 'message': 'No active notification preferences found.' - }, status=status.HTTP_404_NOT_FOUND) - - # Get structured preferences from NotificationAppManager. - structured_preferences = ( - NotificationAppManager() - .get_notification_app_preferences() - ) - - for app_name, app_settings in structured_preferences.items(): - notification_types = app_settings.get( - 'notification_types', {} - ) - - # Process notification types. - for type_name, type_details in notification_types.items(): - if type_name == 'core': - if structured_preferences[app_name][ - 'core_notification_types' - ]: - notification_type = ( - structured_preferences[app_name] - ['core_notification_types'][0] - ) - else: - notification_type = 'core' - user_pref = user_preferences_map.get( - notification_type - ) - else: - user_pref = user_preferences_map.get(type_name) - if user_pref: - # Update dictionary for this type. - type_details['web'] = user_pref.web - 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 - ) - structured_preferences = add_non_editable_in_preference( - add_info_to_notification_config(structured_preferences) - ) + user_preferences_qs = NotificationPreference.objects.filter(user=request.user) + user_preferences_map = {pref.type: pref for pref in user_preferences_qs} + + # Ensure all notification types are present in the user's preferences. + # If any are missing, create them with default values. + diff = set(COURSE_NOTIFICATION_TYPES.keys()) - set(user_preferences_map.keys()) + missing_types = [] + for missing_type in diff: + new_pref = create_notification_preference( + user_id=request.user.id, + notification_type=missing_type, + + ) + missing_types.append(new_pref) + user_preferences_map[missing_type] = new_pref + if missing_types: + NotificationPreference.objects.bulk_create(missing_types) logger.info( - 'Retrieved notification preferences for user %s', - request.user.id, + 'Created %d missing notification preferences for user %s', + len(missing_types), + request.user.id ) - return Response({ - 'status': 'success', - 'message': 'Notification preferences retrieved successfully.', - 'show_preferences': get_show_notifications_tray( - self.request.user - ), - 'data': structured_preferences - }, status=status.HTTP_200_OK) - except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: - logger.error( - 'Failed to retrieve notification preferences for user %s: %s', - getattr(request.user, 'id', None), - str(exc), + + # If no user preferences are found, return an error response. + if not user_preferences_map: + logger.warning( + 'No active notification preferences for user %s', + request.user.id ) return Response({ 'status': 'error', - 'message': 'Failed to retrieve notification preferences.' - }, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + 'message': 'No active notification preferences found for this user.' + }, status=status.HTTP_404_NOT_FOUND) + + # Get the structured preferences from the NotificationAppManager. + # This will include all apps and their notification types. + structured_preferences = NotificationAppManager().get_notification_app_preferences() + + for app_name, app_settings in structured_preferences.items(): + notification_types = app_settings.get('notification_types', {}) + + # Process all notification types (core and non-core) in a single loop. + for type_name, type_details in notification_types.items(): + if type_name == 'core': + if structured_preferences[app_name]['core_notification_types']: + # If the app has core notification types, use the first one as the type name. + # This assumes that the first core notification type is representative of the core settings. + notification_type = structured_preferences[app_name]['core_notification_types'][0] + else: + notification_type = 'core' + user_pref = user_preferences_map.get(notification_type) + else: + user_pref = user_preferences_map.get(type_name) + if user_pref: + # If a preference exists, update the dictionary for this type. + # This directly modifies the 'type_details' dictionary. + type_details['web'] = user_pref.web + 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) + structured_preferences = add_non_editable_in_preference( + add_info_to_notification_config(structured_preferences) + ) + logger.debug( + 'Retrieved notification preferences for user %s', + request.user.id, + ) + return Response({ + 'status': 'success', + 'message': 'Notification preferences retrieved successfully.', + 'show_preferences': get_show_notifications_tray(self.request.user), + 'data': structured_preferences + }, status=status.HTTP_200_OK) def put(self, request): """ - Update notification preferences for authenticated user. + Handles PUT requests to update notification preferences. + + This method updates the user's notification preferences based on the + provided data in the request body. It expects a dictionary with + notification types and their settings. Returns: - Response: DRF Response indicating success or failure. + Response: A DRF Response object indicating success or failure. """ - try: - # Validate incoming data - serializer = UserNotificationPreferenceUpdateAllSerializer( - data=request.data - ) - if not serializer.is_valid(): - logger.warning( - 'Invalid serializer data for user %s: %s', - request.user.id, - serializer.errors - ) - return Response({ - 'status': 'error', - 'message': serializer.errors - }, status=status.HTTP_400_BAD_REQUEST) - - # Get validated data - validated_data = serializer.validated_data - - # Build query set based on notification type - query_set = NotificationPreference.objects.filter( - user_id=request.user.id - ) - - if validated_data['notification_type'] == 'core': - # Get core notification types for the app - __, core_types = NotificationTypeManager( - ).get_notification_app_preference( - notification_app=validated_data['notification_app'] - ) - query_set = query_set.filter(type__in=core_types) - else: - # Filter by single notification type - query_set = query_set.filter( - type=validated_data['notification_type'] - ) - - # Prepare update data - updated_data = self._prepare_update_data(validated_data) - - # Update preferences - update_count = query_set.update(**updated_data) - - # Log the event - self._log_preference_update_event(request.user, validated_data) - logger.info( - 'Updated %d notification preferences for user %s with app=%s', - update_count, + # Validate incoming data + serializer = UserNotificationPreferenceUpdateAllSerializer(data=request.data) + if not serializer.is_valid(): + invalid_fields = list(serializer.errors.keys()) + logger.warning( + 'Invalid serializer data for user %s; invalid fields: %s', request.user.id, - validated_data["notification_app"], + invalid_fields, ) - # Prepare and return response - response_data = self._prepare_response_data(validated_data) - return Response(response_data, status=status.HTTP_200_OK) - except (NotificationPreference.DoesNotExist, KeyError, AttributeError, TypeError) as exc: - logger.error( - 'Failed to update notification preferences for user %s: %s', - getattr(request.user, 'id', None), - str(exc), + logger.debug( + 'Serializer validation errors for user %s: %s', + request.user.id, + serializer.errors, ) return Response({ 'status': 'error', - 'message': 'Failed to update notification preferences.' - }, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + 'message': serializer.errors + }, status=status.HTTP_400_BAD_REQUEST) + + # Get validated data for easier access + validated_data = serializer.validated_data + + # Build query set based on notification type + query_set = NotificationPreference.objects.filter(user_id=request.user.id) + + if validated_data['notification_type'] == 'core': + # Get core notification types for the app + __, core_types = NotificationTypeManager().get_notification_app_preference( + notification_app=validated_data['notification_app'] + ) + query_set = query_set.filter(type__in=core_types) + else: + # Filter by single notification type + query_set = query_set.filter(type=validated_data['notification_type']) + + # Prepare update data based on channel type + updated_data = self._prepare_update_data(validated_data) + + # Update preferences + update_count = query_set.update(**updated_data) + + # Log the event + self._log_preference_update_event(request.user, validated_data) + logger.info( + 'Updated %d notification preferences for user %s with app=%s', + update_count, + request.user.id, + validated_data["notification_app"], + ) + + # Prepare and return response + response_data = self._prepare_response_data(validated_data) + return Response(response_data, status=status.HTTP_200_OK) def _prepare_update_data(self, validated_data): """ - Prepare the data dictionary for updating preferences. + Prepare the data dictionary for updating notification preferences. Args: validated_data (dict): Validated serializer data @@ -648,22 +469,12 @@ def _prepare_update_data(self, validated_data): Returns: dict: Dictionary with update data """ - try: - channel = validated_data['notification_channel'] + channel = validated_data['notification_channel'] - if channel == 'email_cadence': - result = {channel: validated_data['email_cadence']} - else: - result = {channel: validated_data['value']} - logger.debug( - 'Prepared update data for channel %s: %s', - channel, - result, - ) - return result - except KeyError as exc: - logger.error('Failed to prepare update data: %s', str(exc)) - raise + if channel == 'email_cadence': + return {channel: validated_data['email_cadence']} + else: + return {channel: validated_data['value']} def _log_preference_update_event(self, user, validated_data): """ @@ -673,27 +484,14 @@ def _log_preference_update_event(self, user, validated_data): user: The user making the update validated_data (dict): Validated serializer data """ - try: - event_data = { - 'notification_app': validated_data['notification_app'], - 'notification_type': validated_data['notification_type'], - 'notification_channel': validated_data[ - 'notification_channel' - ], - 'value': validated_data.get('value'), - 'email_cadence': validated_data.get('email_cadence'), - } - notification_preference_update_event(user, [], event_data) - logger.debug( - 'Logged preference update events for user %s', - user.id - ) - except (KeyError, AttributeError, TypeError, ValueError) as exc: - logger.error( - 'Failed to log preference update events for user %s: %s', - user.id, - str(exc) - ) + event_data = { + 'notification_app': validated_data['notification_app'], + 'notification_type': validated_data['notification_type'], + 'notification_channel': validated_data['notification_channel'], + 'value': validated_data.get('value'), + 'email_cadence': validated_data.get('email_cadence'), + } + notification_preference_update_event(user, [], event_data) def _prepare_response_data(self, validated_data): """ @@ -707,10 +505,7 @@ def _prepare_response_data(self, validated_data): """ email_cadence = validated_data.get('email_cadence', None) # Determine the updated value - updated_value = validated_data.get( - 'value', - email_cadence if email_cadence else None - ) + updated_value = validated_data.get('value', email_cadence if email_cadence else None) # Determine the channel channel = validated_data.get('notification_channel') @@ -720,9 +515,7 @@ def _prepare_response_data(self, validated_data): return { 'status': 'success', 'message': 'Notification preferences update completed', - 'show_preferences': get_show_notifications_tray( - self.request.user - ), + 'show_preferences': get_show_notifications_tray(self.request.user), 'data': { 'updated_value': updated_value, 'notification_type': validated_data['notification_type'],