Skip to content

fix: infer email_cadence channel for v3 notification preference PUTs #39241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvanhorn wants to merge 1 commit into from
+118 −8
Closed

fix: infer email_cadence channel for v3 notification preference PUTs #39241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions openedx/core/djangoapps/notifications/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 37 additions & 8 deletions openedx/core/djangoapps/notifications/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand All @@ -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.
Expand All @@ -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']}
Expand All @@ -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'),
}
Expand All @@ -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',
Expand Down