Skip to content
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.

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

feat: added notification grouping #35368

Merged
merged 16 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def send_new_comment_notification(self):
"author_name": str(author_name),
"author_pronoun": str(author_pronoun),
"email_content": clean_thread_html_body(self.comment.body),
"group_by_id": self.parent_response.id
}
self._send_notification([self.thread.user_id], "new_comment", extra_context=context)

Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/discussion/rest_api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ def test_send_notification_to_parent_threads(self):
'replier_name': self.user_3.username,
'post_title': self.thread.title,
'email_content': self.comment.body,
'group_by_id': self.thread_2.id,
'author_name': 'dummy\'s',
'author_pronoun': 'dummy\'s',
'course_name': self.course.display_name,
Expand Down Expand Up @@ -438,6 +439,7 @@ def test_comment_creators_own_response(self):
expected_context = {
'replier_name': self.user_3.username,
'post_title': self.thread.title,
'group_by_id': self.thread_2.id,
'author_name': 'dummy\'s',
'author_pronoun': 'your',
'course_name': self.course.display_name,
Expand Down
46 changes: 36 additions & 10 deletions openedx/core/djangoapps/notifications/base_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
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
from .notification_content import get_notification_type_content_function
from .notification_content import get_notification_type_context_function

FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role'


COURSE_NOTIFICATION_TYPES = {
'new_comment_on_response': {
'notification_app': 'discussion',
Expand All @@ -32,6 +31,8 @@
'is_core': True,
'content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on <{strong}>{author_name}'
'</{strong}> response to your post <{strong}>{post_title}</{strong}></{p}>'),
'grouped_content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on <{strong}>{author_name}'
'</{strong}> response to your post <{strong}>{post_title}</{strong}></{p}>'),
'content_context': {
'post_title': 'Post title',
'author_name': 'author name',
Expand Down Expand Up @@ -481,21 +482,46 @@ def get_notification_app_preferences(self):
def get_notification_content(notification_type, context):
"""
Returns notification content for the given notification type with provided context.

Args:
notification_type (str): The type of notification (e.g., 'course_update').
context (dict): The context data to be used in the notification template.

Returns:
str: Rendered notification content based on the template and context.
"""
html_tags_context = {
context.update({
'strong': 'strong',
'p': 'p',
}
content_function = get_notification_type_content_function(notification_type)
})

# Retrieve the function associated with the notification type.
context_function = get_notification_type_context_function(notification_type)

# Fix a specific case where 'course_update' needs to be renamed to 'course_updates'.
if notification_type == 'course_update':
notification_type = 'course_updates'

# Retrieve the notification type object from NotificationTypeManager.
notification_type = NotificationTypeManager().notification_types.get(notification_type, None)

if notification_type:
if content_function:
return content_function(notification_type, context)
notification_type_content_template = notification_type.get('content_template', None)
if notification_type_content_template:
return notification_type_content_template.format(**context, **html_tags_context)
# Check if the notification is grouped.
is_grouped = context.get('grouped', False)

# Determine the correct template key based on whether it's grouped or not.
template_key = "grouped_content_template" if is_grouped else "content_template"

# Get the corresponding template from the notification type.
template = notification_type.get(template_key, None)

# Apply the context function to transform or modify the context.
context = context_function(context)

if template:
# Handle grouped templates differently by modifying the context using a different function.
return template.format(**context)

return ''


Expand Down
11 changes: 11 additions & 0 deletions openedx/core/djangoapps/notifications/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,14 @@
# .. toggle_target_removal_date: 2024-10-10
# .. toggle_tickets: INF-1304
ENABLE_ORA_GRADE_NOTIFICATION = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_ora_grade_notifications", __name__)

# .. toggle_name: notifications.enable_notification_grouping
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable the Notifications Grouping feature
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2024-07-22
# .. toggle_target_removal_date: 2025-06-01
# .. toggle_warning: When the flag is ON, Notifications Grouping feature is enabled.
# .. toggle_tickets: INF-1472
ENABLE_NOTIFICATION_GROUPING = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_notification_grouping', __name__)
121 changes: 121 additions & 0 deletions openedx/core/djangoapps/notifications/grouping_notifications.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
"""
Notification grouping utilities for notifications
"""
import datetime
from typing import Dict, Type, Union

from pytz import utc

from abc import ABC, abstractmethod

from openedx.core.djangoapps.notifications.models import Notification


class BaseNotificationGrouper(ABC):
"""
Base class for notification groupers.
"""

@abstractmethod
def group(self, new_notification, old_notification):
pass


class NotificationRegistry:
"""
Registry for notification groupers.
"""
_groupers: Dict[str, Type[BaseNotificationGrouper]] = {}

@classmethod
def register(cls, notification_type: str):
"""
Registers a notification grouper for the given notification type.
Args
notification_type: The type of notification for which to register the grouper.

Returns:
A decorator that registers the grouper class for the given notification type.
"""

def decorator(grouper_class: Type[BaseNotificationGrouper]) -> Type[BaseNotificationGrouper]:
"""
Registers the grouper class for the given notification type.
"""
cls._groupers[notification_type] = grouper_class
return grouper_class

return decorator

@classmethod
def get_grouper(cls, notification_type: str) -> Union[BaseNotificationGrouper, None]:
"""Retrieves the appropriate notification grouper based on the given notification type.

Args:
notification_type: The type of notification for which to retrieve the grouper.

Returns:
The corresponding BaseNotificationGrouper instance or None if no grouper is found.
"""
grouper_class = cls._groupers.get(notification_type)
if not grouper_class:
return None
return grouper_class()


@NotificationRegistry.register('new_comment')
class NewCommentGrouper(BaseNotificationGrouper):
"""
Groups new comment notifications based on the replier name.
"""

def group(self, new_notification, old_notification):
"""
Groups new comment notifications based on the replier name.
"""
context = old_notification.content_context.copy()
if not context.get('grouped'):
context['replier_name_list'] = [context['replier_name']]
context['grouped_count'] = 1
context['grouped'] = True
context['replier_name_list'].append(new_notification.content_context['replier_name'])
context['grouped_count'] += 1
return context


def group_user_notifications(new_notification: Notification, old_notification: Notification):
"""
Groups user notification based on notification type and group_id
"""
notification_type = new_notification.notification_type
grouper_class = NotificationRegistry.get_grouper(notification_type)

if grouper_class:
old_notification.content_context = grouper_class.group(new_notification, old_notification)
old_notification.content_context['grouped'] = True
old_notification.web = old_notification.web or new_notification.web
old_notification.email = old_notification.email or new_notification.email
old_notification.last_read = None
old_notification.last_seen = None
old_notification.created = utc.localize(datetime.datetime.now())
old_notification.save()


def get_user_existing_notifications(user_ids, notification_type, group_by_id, course_id):
"""
Returns user last group able notification
"""
notifications = Notification.objects.filter(
user__in=user_ids,
notification_type=notification_type,
group_by_id=group_by_id,
course_id=course_id
)
notifications_mapping = {user_id: [] for user_id in user_ids}
for notification in notifications:
notifications_mapping[notification.user_id].append(notification)

for user_id, notifications in notifications_mapping.items():
notifications.sort(key=lambda elem: elem.created)
notifications_mapping[user_id] = notifications[0] if notifications else None
return notifications_mapping
36 changes: 24 additions & 12 deletions openedx/core/djangoapps/notifications/notification_content.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,47 @@
"""
Helper functions for overriding notification content for given notification type.
"""
from typing import Dict


def get_notification_type_content_function(notification_type):
def get_notification_type_context_function(notification_type) -> callable:
"""
Returns the content function for the given notification if it exists.
Returns:
callable : The function that returns the context for the given notification type.
"""
try:
return globals()[f"get_{notification_type}_notification_content"]
return globals()[f"get_{notification_type}_notification_context"]
except KeyError:
return None
return lambda context: context


def get_notification_content_with_author_pronoun(notification_type, context):
def get_notification_context_with_author_pronoun(context: Dict) -> Dict:
"""
Helper function to get notification content with author's pronoun.
Returns the context for the given notification type with the author pronoun.

"""
html_tags_context = {
'strong': 'strong',
'p': 'p',
}
notification_type_content_template = notification_type.get('content_template', None)
context.update(html_tags_context)
if 'author_pronoun' in context:
context['author_name'] = context['author_pronoun']
if notification_type_content_template:
return notification_type_content_template.format(**context, **html_tags_context)
return ''
return context


# Returns notification content for the new_comment notification.
get_new_comment_notification_content = get_notification_content_with_author_pronoun
def get_new_comment_notification_context(context):
"""
Returns the context for the new_comment notification
"""
if not context.get('grouped'):
return get_notification_context_with_author_pronoun(context)
num_repliers = context['grouped_count']
repliers_string = f"{num_repliers - 1} other{'s' if num_repliers > 2 else ''}"
context['replier_name'] = f"{context['replier_name_list'][0]} and {repliers_string}"
return context


# Returns notification content for the comment_on_followed_post notification.
get_comment_on_followed_post_notification_content = get_notification_content_with_author_pronoun
get_comment_on_followed_post_notification_context = get_notification_context_with_author_pronoun
45 changes: 30 additions & 15 deletions openedx/core/djangoapps/notifications/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@
from pytz import UTC

from common.djangoapps.student.models import CourseEnrollment
from openedx.core.djangoapps.notifications.audience_filters import NotificationFilter
from openedx.core.djangoapps.notifications.base_notification import (
get_default_values_of_preference,
get_notification_content
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATION_GROUPING, ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.events import notification_generated_event
from openedx.core.djangoapps.notifications.audience_filters import NotificationFilter
from openedx.core.djangoapps.notifications.grouping_notifications import (
get_user_existing_notifications,
group_user_notifications, NotificationRegistry,
)
from openedx.core.djangoapps.notifications.models import (
CourseNotificationPreference,
Notification,
get_course_notification_preference_config_version
)
from openedx.core.djangoapps.notifications.utils import clean_arguments, get_list_in_batches


logger = get_task_logger(__name__)


Expand Down Expand Up @@ -124,7 +129,10 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c

user_ids = list(set(user_ids))
batch_size = settings.NOTIFICATION_CREATION_BATCH_SIZE

group_by_id = context.pop('group_by_id', '')
grouping_function = NotificationRegistry.get_grouper(notification_type)
waffle_flag_enabled = ENABLE_NOTIFICATION_GROUPING.is_enabled(course_key)
grouping_enabled = waffle_flag_enabled and group_by_id and grouping_function is not None
notifications_generated = False
notification_content = ''
sender_id = context.pop('sender_id', None)
Expand All @@ -136,6 +144,10 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type)
logger.debug(f'After applying filters, sending notifications to {len(batch_user_ids)} users in {course_key}')

existing_notifications = (
get_user_existing_notifications(batch_user_ids, notification_type, group_by_id, course_key))\
if grouping_enabled else {}

# check if what is preferences of user and make decision to send notification or not
preferences = CourseNotificationPreference.objects.filter(
user_id__in=batch_user_ids,
Expand All @@ -160,19 +172,22 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
preference.get_app_config(app_name).get('enabled', False)
):
notification_preferences = preference.get_channels_for_notification_type(app_name, notification_type)
notifications.append(
Notification(
user_id=user_id,
app_name=app_name,
notification_type=notification_type,
content_context=context,
content_url=content_url,
course_id=course_key,
web='web' in notification_preferences,
email='email' in notification_preferences,
)
new_notification = Notification(
user_id=user_id,
app_name=app_name,
notification_type=notification_type,
content_context=context,
content_url=content_url,
course_id=course_key,
web='web' in notification_preferences,
email='email' in notification_preferences,
group_by_id=group_by_id,
)
generated_notification_audience.append(user_id)
if grouping_enabled and existing_notifications.get(user_id, None):
group_user_notifications(new_notification, existing_notifications[user_id])
else:
notifications.append(new_notification)
generated_notification_audience.append(user_id)

# send notification to users but use bulk_create
notification_objects = Notification.objects.bulk_create(notifications)
Expand Down
Loading
Loading