From 5f8e6c9c038c5c7c67288e431fd07116f88e96f6 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:32:10 -0700 Subject: [PATCH] fix: infer email_cadence channel for v3 notification preference PUTs The view now resolves one channel for the request: it keeps an explicit channel, and treats a body that only supplies a cadence as the email_cadence channel. That value is used for the preference update, the analytics event, and the response. A cadence-only update is rejected with 400 when that notification type's email channel is not editable. When the resolved channel is email_cadence, the response reports the cadence, including when the body also contains a boolean value. Fixes #39221 --- .../notifications/tests/test_views.py | 81 +++++++++++++++++++ .../core/djangoapps/notifications/views.py | 45 +++++++++-- 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index c4a28bbf0461..1a4d4f3b1435 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -871,6 +871,87 @@ def test_update_preferences_grouped(self): self.assertEqual(len(set(cadence_set)), 1) # noqa: PT009 self.assertIn('Weekly', set(cadence_set)) # noqa: PT009 + @mock.patch('openedx.core.djangoapps.notifications.views.notification_preference_update_event') + def test_update_preferences_grouped_email_cadence_without_channel(self, mock_preference_update_event): + """ + A cadence-only grouped update infers the email_cadence channel. + """ + update_data = { + "notification_app": "discussion", + "notification_type": "grouped_notification", + "email_cadence": "Weekly" + } + grouped_types = [ + key + for key, type_config in get_default_values_of_preferences().items() + if type_config.get('use_app_defaults') + ] + self.client.get(self.url) + response = self.client.put(self.url, update_data, format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(response.data['status'], 'success') # noqa: PT009 + self.assertEqual(response.data['data']['channel'], 'email_cadence') # noqa: PT009 + cadence_set = NotificationPreference.objects.filter(user=self.user, type__in=grouped_types).values_list( + 'email_cadence', flat=True + ) + self.assertEqual(len(set(cadence_set)), 1) # noqa: PT009 + self.assertIn('Weekly', set(cadence_set)) # noqa: PT009 + + mock_preference_update_event.assert_called_once() + event_data = mock_preference_update_event.call_args[0][2] + self.assertEqual(event_data['notification_channel'], 'email_cadence') # noqa: PT009 + self.assertEqual(event_data['email_cadence'], 'Weekly') # noqa: PT009 + + @mock.patch( + 'openedx.core.djangoapps.notifications.email.utils.get_default_values_of_preferences', + return_value={'new_discussion_post': {'non_editable': ['email']}} + ) + def test_update_preferences_email_cadence_without_channel_when_email_is_not_editable(self, mock_get_defaults): + """ + A cadence-only update is rejected when the notification type's email channel is not editable. + """ + update_data = { + "notification_app": "discussion", + "notification_type": "new_discussion_post", + "email_cadence": "Weekly" + } + self.client.get(self.url) + response = self.client.put(self.url, update_data, format='json') + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) # noqa: PT009 + self.assertIn('notification_channel', response.data['message']) # noqa: PT009 + preference = NotificationPreference.objects.get(type='new_discussion_post', user=self.user) + self.assertEqual(preference.email_cadence, 'Daily') # noqa: PT009 + + def test_update_preferences_email_cadence_response_value_without_channel(self): + """ + A cadence-only update reports the cadence even when a boolean value is also supplied. + """ + update_data = { + "notification_app": "discussion", + "notification_type": "new_discussion_post", + "value": False, + "email_cadence": "Weekly" + } + self.client.get(self.url) + response = self.client.put(self.url, update_data, format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(response.data['data']['updated_value'], 'Weekly') # noqa: PT009 + preference = NotificationPreference.objects.get(type='new_discussion_post', user=self.user) + self.assertEqual(preference.email_cadence, 'Weekly') # noqa: PT009 + + def test_update_preferences_without_channel_or_cadence(self): + """ + An update with neither a channel nor a cadence is rejected by the serializer. + """ + update_data = { + "notification_app": "discussion", + "notification_type": "new_discussion_post", + } + response = self.client.put(self.url, update_data, format='json') + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) # noqa: PT009 + self.assertEqual(response.data['status'], 'error') # noqa: PT009 + self.assertIn('notification_channel', response.data['message']) # noqa: PT009 + def test_update_preferences(self): """ Test case: Update notification preferences for the authenticated user diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 3d786da14d7f..254ab39d9087 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -20,6 +20,7 @@ from openedx.core.djangoapps.notifications.permissions import allow_any_authenticated_user from .base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES, filter_notification_types_by_app +from .email.utils import is_notification_type_channel_editable from .events import ( notification_preference_update_event, notification_read_event, @@ -332,6 +333,21 @@ def put(self, request): # Get validated data for easier access validated_data = serializer.validated_data + if ( + not validated_data.get('notification_channel') + and validated_data.get('email_cadence') + and not is_notification_type_channel_editable(validated_data['notification_type'], 'email') + ): + return Response({ + 'status': 'error', + 'message': { + 'notification_channel': [ + 'email_cadence is not editable for notification type ' + f"{validated_data['notification_type']}." + ] + } + }, status=status.HTTP_400_BAD_REQUEST) + # Build query set based on notification type query_set = NotificationPreference.objects.filter(user_id=request.user.id) @@ -355,6 +371,22 @@ def put(self, request): response_data = self._prepare_response_data(validated_data) return Response(response_data, status=status.HTTP_200_OK) + def _get_effective_notification_channel(self, validated_data): + """ + Honor an explicit notification_channel, otherwise infer email_cadence + when a cadence value is supplied. + + Args: + validated_data (dict): Validated serializer data + + Returns: + str or None: Channel used for the update, event, and response + """ + channel = validated_data.get('notification_channel') + if not channel and validated_data.get('email_cadence'): + channel = 'email_cadence' + return channel + def _prepare_update_data(self, validated_data): """ Prepare the data dictionary for updating notification preferences. @@ -365,7 +397,7 @@ def _prepare_update_data(self, validated_data): Returns: dict: Dictionary with update data """ - channel = validated_data['notification_channel'] + channel = self._get_effective_notification_channel(validated_data) if channel == 'email_cadence': return {channel: validated_data['email_cadence']} @@ -383,7 +415,7 @@ def _log_preference_update_event(self, user, validated_data): event_data = { 'notification_app': validated_data['notification_app'], 'notification_type': validated_data['notification_type'], - 'notification_channel': validated_data['notification_channel'], + 'notification_channel': self._get_effective_notification_channel(validated_data), 'value': validated_data.get('value'), 'email_cadence': validated_data.get('email_cadence'), } @@ -400,13 +432,10 @@ def _prepare_response_data(self, validated_data): dict: Response data dictionary """ 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) + channel = self._get_effective_notification_channel(validated_data) - # Determine the channel - channel = validated_data.get('notification_channel') - if not channel and validated_data.get('email_cadence'): - channel = 'email_cadence' + # Determine the updated value + updated_value = email_cadence if channel == 'email_cadence' else validated_data.get('value') return { 'status': 'success',