feat: Add notify all learners option for discussion post (#36922)
* feat: Add notify all learners option for discussion post * fix: Remove waffle flag from default notification dict
This commit is contained in:
@@ -127,7 +127,8 @@ from .utils import (
|
||||
get_usernames_for_course,
|
||||
get_usernames_from_search_string,
|
||||
set_attribute,
|
||||
is_posting_allowed
|
||||
is_posting_allowed,
|
||||
can_user_notify_all_learners
|
||||
)
|
||||
|
||||
User = get_user_model()
|
||||
@@ -333,6 +334,8 @@ def get_course(request, course_key, check_tab=True):
|
||||
course.get_discussion_blackout_datetimes()
|
||||
)
|
||||
discussion_tab = CourseTabList.get_tab_by_type(course.tabs, 'discussion')
|
||||
is_course_staff = CourseStaffRole(course_key).has_user(request.user)
|
||||
is_course_admin = CourseInstructorRole(course_key).has_user(request.user)
|
||||
return {
|
||||
"id": str(course_key),
|
||||
"is_posting_enabled": is_posting_enabled,
|
||||
@@ -358,8 +361,8 @@ def get_course(request, course_key, check_tab=True):
|
||||
}),
|
||||
"is_group_ta": bool(user_roles & {FORUM_ROLE_GROUP_MODERATOR}),
|
||||
"is_user_admin": request.user.is_staff,
|
||||
"is_course_staff": CourseStaffRole(course_key).has_user(request.user),
|
||||
"is_course_admin": CourseInstructorRole(course_key).has_user(request.user),
|
||||
"is_course_staff": is_course_staff,
|
||||
"is_course_admin": is_course_admin,
|
||||
"provider": course_config.provider_type,
|
||||
"enable_in_context": course_config.enable_in_context,
|
||||
"group_at_subsection": course_config.plugin_configuration.get("group_at_subsection", False),
|
||||
@@ -372,6 +375,9 @@ def get_course(request, course_key, check_tab=True):
|
||||
for (reason_code, label) in CLOSE_REASON_CODES.items()
|
||||
],
|
||||
'show_discussions': bool(discussion_tab and discussion_tab.is_enabled(course, request.user)),
|
||||
'is_notify_all_learners_enabled': can_user_notify_all_learners(
|
||||
course_key, user_roles, is_course_staff, is_course_admin
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
@@ -1469,6 +1475,8 @@ def create_thread(request, thread_data):
|
||||
if not discussion_open_for_user(course, user):
|
||||
raise DiscussionBlackOutException
|
||||
|
||||
notify_all_learners = thread_data.pop("notify_all_learners", False)
|
||||
|
||||
context = get_context(course, request)
|
||||
_check_initializable_thread_fields(thread_data, context)
|
||||
discussion_settings = CourseDiscussionSettings.get(course_key)
|
||||
@@ -1484,7 +1492,7 @@ def create_thread(request, thread_data):
|
||||
raise ValidationError(dict(list(serializer.errors.items()) + list(actions_form.errors.items())))
|
||||
serializer.save()
|
||||
cc_thread = serializer.instance
|
||||
thread_created.send(sender=None, user=user, post=cc_thread)
|
||||
thread_created.send(sender=None, user=user, post=cc_thread, notify_all_learners=notify_all_learners)
|
||||
api_thread = serializer.data
|
||||
_do_extra_actions(api_thread, cc_thread, list(thread_data.keys()), actions_form, context, request)
|
||||
|
||||
|
||||
@@ -318,17 +318,18 @@ class DiscussionNotificationSender:
|
||||
self._populate_context_with_ids_for_mobile(context, notification_type)
|
||||
self._send_notification([self.creator.id], notification_type, extra_context=context)
|
||||
|
||||
def send_new_thread_created_notification(self):
|
||||
def send_new_thread_created_notification(self, notify_all_learners=False):
|
||||
"""
|
||||
Send notification based on notification_type
|
||||
"""
|
||||
thread_type = self.thread.attributes['thread_type']
|
||||
notification_type = (
|
||||
|
||||
notification_type = "new_instructor_all_learners_post" if notify_all_learners else (
|
||||
"new_question_post"
|
||||
if thread_type == "question"
|
||||
else ("new_discussion_post" if thread_type == "discussion" else "")
|
||||
)
|
||||
if notification_type not in ['new_discussion_post', 'new_question_post']:
|
||||
if notification_type not in ['new_discussion_post', 'new_question_post', 'new_instructor_all_learners_post']:
|
||||
raise ValueError(f'Invalid notification type {notification_type}')
|
||||
|
||||
audience_filters = self._create_cohort_course_audience()
|
||||
|
||||
@@ -6,8 +6,11 @@ from django.contrib.auth import get_user_model
|
||||
from edx_django_utils.monitoring import set_code_owner_attribute
|
||||
from opaque_keys.edx.locator import CourseKey
|
||||
|
||||
from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole
|
||||
from lms.djangoapps.courseware.courses import get_course_with_access
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names
|
||||
from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
|
||||
from lms.djangoapps.discussion.rest_api.utils import can_user_notify_all_learners
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client import Comment
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
|
||||
@@ -17,7 +20,7 @@ User = get_user_model()
|
||||
|
||||
@shared_task
|
||||
@set_code_owner_attribute
|
||||
def send_thread_created_notification(thread_id, course_key_str, user_id):
|
||||
def send_thread_created_notification(thread_id, course_key_str, user_id, notify_all_learners=False):
|
||||
"""
|
||||
Send notification when a new thread is created
|
||||
"""
|
||||
@@ -26,9 +29,17 @@ def send_thread_created_notification(thread_id, course_key_str, user_id):
|
||||
return
|
||||
thread = Thread(id=thread_id).retrieve()
|
||||
user = User.objects.get(id=user_id)
|
||||
|
||||
if notify_all_learners:
|
||||
is_course_staff = CourseStaffRole(course_key).has_user(user)
|
||||
is_course_admin = CourseInstructorRole(course_key).has_user(user)
|
||||
user_roles = get_user_role_names(user, course_key)
|
||||
if not can_user_notify_all_learners(course_key, user_roles, is_course_staff, is_course_admin):
|
||||
return
|
||||
|
||||
course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True)
|
||||
notification_sender = DiscussionNotificationSender(thread, course, user)
|
||||
notification_sender.send_new_thread_created_notification()
|
||||
notification_sender.send_new_thread_created_notification(notify_all_learners)
|
||||
|
||||
|
||||
@shared_task
|
||||
|
||||
@@ -214,6 +214,7 @@ class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase)
|
||||
'edit_reasons': [{'code': 'test-edit-reason', 'label': 'Test Edit Reason'}],
|
||||
'post_close_reasons': [{'code': 'test-close-reason', 'label': 'Test Close Reason'}],
|
||||
'show_discussions': True,
|
||||
'is_notify_all_learners_enabled': False
|
||||
}
|
||||
|
||||
@ddt.data(
|
||||
@@ -1379,7 +1380,9 @@ class CreateThreadTest(
|
||||
"read": True,
|
||||
})
|
||||
self.register_post_thread_response(cs_thread)
|
||||
with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)):
|
||||
with self.assert_signal_sent(
|
||||
api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners')
|
||||
):
|
||||
actual = create_thread(self.request, self.minimal_data)
|
||||
expected = self.expected_thread_data({
|
||||
"id": "test_id",
|
||||
@@ -1445,7 +1448,9 @@ class CreateThreadTest(
|
||||
|
||||
_assign_role_to_user(user=self.user, course_id=self.course.id, role=FORUM_ROLE_MODERATOR)
|
||||
|
||||
with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)):
|
||||
with self.assert_signal_sent(
|
||||
api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners')
|
||||
):
|
||||
actual = create_thread(self.request, self.minimal_data)
|
||||
expected = self.expected_thread_data({
|
||||
"author_label": "Moderator",
|
||||
@@ -1517,7 +1522,9 @@ class CreateThreadTest(
|
||||
"read": True,
|
||||
})
|
||||
self.register_post_thread_response(cs_thread)
|
||||
with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)):
|
||||
with self.assert_signal_sent(
|
||||
api, 'thread_created', sender=None, user=self.user, exclude_args=('post', 'notify_all_learners')
|
||||
):
|
||||
create_thread(self.request, data)
|
||||
event_name, event_data = mock_emit.call_args[0]
|
||||
assert event_name == 'edx.forum.thread.created'
|
||||
|
||||
@@ -29,7 +29,7 @@ from openedx.core.djangoapps.django_comment_common.models import (
|
||||
FORUM_ROLE_STUDENT,
|
||||
CourseDiscussionSettings
|
||||
)
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
@@ -190,29 +190,46 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
|
||||
"""
|
||||
|
||||
@ddt.data(
|
||||
('new_question_post',),
|
||||
('new_discussion_post',),
|
||||
('new_question_post', False, False),
|
||||
('new_discussion_post', False, False),
|
||||
('new_discussion_post', True, True),
|
||||
('new_discussion_post', True, False),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_notification_is_send_to_all_enrollments(self, notification_type):
|
||||
def test_notification_is_send_to_all_enrollments(
|
||||
self, notification_type, notify_all_learners, waffle_flag_enabled
|
||||
):
|
||||
"""
|
||||
Tests notification is sent to all users if course is not cohorted
|
||||
"""
|
||||
self._assign_enrollments()
|
||||
thread_type = (
|
||||
"discussion"
|
||||
if notification_type == "new_discussion_post"
|
||||
else ("question" if notification_type == "new_question_post" else "")
|
||||
"discussion" if notification_type == "new_discussion_post" else "question"
|
||||
)
|
||||
thread = self._create_thread(thread_type=thread_type)
|
||||
handler = mock.Mock()
|
||||
COURSE_NOTIFICATION_REQUESTED.connect(handler)
|
||||
send_thread_created_notification(thread['id'], str(self.course.id), self.author.id)
|
||||
self.assertEqual(handler.call_count, 1)
|
||||
course_notification_data = handler.call_args[1]['course_notification_data']
|
||||
assert notification_type == course_notification_data.notification_type
|
||||
notification_audience_filters = {}
|
||||
assert notification_audience_filters == course_notification_data.audience_filters
|
||||
|
||||
with override_waffle_flag(ENABLE_NOTIFY_ALL_LEARNERS, active=waffle_flag_enabled):
|
||||
thread = self._create_thread(thread_type=thread_type)
|
||||
handler = mock.Mock()
|
||||
COURSE_NOTIFICATION_REQUESTED.connect(handler)
|
||||
|
||||
send_thread_created_notification(
|
||||
thread['id'],
|
||||
str(self.course.id),
|
||||
self.author.id,
|
||||
notify_all_learners
|
||||
)
|
||||
expected_handler_calls = 0 if notify_all_learners and not waffle_flag_enabled else 1
|
||||
self.assertEqual(handler.call_count, expected_handler_calls)
|
||||
|
||||
if handler.call_count:
|
||||
course_notification_data = handler.call_args[1]['course_notification_data']
|
||||
expected_type = (
|
||||
'new_instructor_all_learners_post'
|
||||
if notify_all_learners and waffle_flag_enabled
|
||||
else notification_type
|
||||
)
|
||||
self.assertEqual(course_notification_data.notification_type, expected_type)
|
||||
self.assertEqual(course_notification_data.audience_filters, {})
|
||||
|
||||
@ddt.data(
|
||||
('cohort_1', 'new_question_post'),
|
||||
|
||||
@@ -557,6 +557,7 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
|
||||
"edit_reasons": [{"code": "test-edit-reason", "label": "Test Edit Reason"}],
|
||||
"post_close_reasons": [{"code": "test-close-reason", "label": "Test Close Reason"}],
|
||||
'show_discussions': True,
|
||||
'is_notify_all_learners_enabled': False
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ from pytz import UTC
|
||||
|
||||
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFY_ALL_LEARNERS
|
||||
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, PostingRestriction
|
||||
from openedx.core.djangoapps.django_comment_common.models import (
|
||||
FORUM_ROLE_ADMINISTRATOR,
|
||||
@@ -379,3 +380,25 @@ def is_posting_allowed(posting_restrictions: str, blackout_schedules: List):
|
||||
return not any(schedule["start"] <= now <= schedule["end"] for schedule in blackout_schedules)
|
||||
else:
|
||||
return False
|
||||
|
||||
|
||||
def can_user_notify_all_learners(course_key, user_roles, is_course_staff, is_course_admin):
|
||||
"""
|
||||
Check if user posting is allowed to notify all learners based on the given restrictions
|
||||
|
||||
Args:
|
||||
course_key (CourseKey): CourseKey for which user creating any discussion post.
|
||||
user_roles (Dict): Roles of the posting user
|
||||
is_course_staff (Boolean): Whether the user has a course staff access.
|
||||
is_course_admin (Boolean): Whether the user has a course admin access.
|
||||
|
||||
Returns:
|
||||
bool: True if posting for all learner is allowed to this user, False otherwise.
|
||||
"""
|
||||
is_staff_or_instructor = any([
|
||||
user_roles.intersection({FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR}),
|
||||
is_course_staff,
|
||||
is_course_admin,
|
||||
])
|
||||
|
||||
return is_staff_or_instructor and ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key)
|
||||
|
||||
@@ -166,7 +166,10 @@ def create_thread_created_notification(*args, **kwargs):
|
||||
"""
|
||||
user = kwargs['user']
|
||||
post = kwargs['post']
|
||||
send_thread_created_notification.apply_async(args=[post.id, post.attributes['course_id'], user.id])
|
||||
notify_all_learners = kwargs.get('notify_all_learners', False)
|
||||
send_thread_created_notification.apply_async(
|
||||
args=[post.id, post.attributes['course_id'], user.id, notify_all_learners]
|
||||
)
|
||||
|
||||
|
||||
@receiver(signals.comment_created)
|
||||
|
||||
@@ -230,6 +230,24 @@ COURSE_NOTIFICATION_TYPES = {
|
||||
'email_template': '',
|
||||
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE],
|
||||
},
|
||||
'new_instructor_all_learners_post': {
|
||||
'notification_app': 'discussion',
|
||||
'name': 'new_instructor_all_learners_post',
|
||||
'is_core': False,
|
||||
'info': '',
|
||||
'web': True,
|
||||
'email': False,
|
||||
'email_cadence': EmailCadence.DAILY,
|
||||
'push': False,
|
||||
'non_editable': [],
|
||||
'content_template': _('<{p}>Your instructor posted <{strong}>{post_title}</{strong}></{p}>'),
|
||||
'grouped_content_template': '',
|
||||
'content_context': {
|
||||
'post_title': 'Post title',
|
||||
},
|
||||
'email_template': '',
|
||||
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
|
||||
},
|
||||
}
|
||||
|
||||
COURSE_NOTIFICATION_APPS = {
|
||||
|
||||
@@ -49,3 +49,13 @@ ENABLE_ORA_GRADE_NOTIFICATION = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_ora
|
||||
# .. 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__)
|
||||
|
||||
# .. toggle_name: notifications.post_enable_notify_all_learners
|
||||
# .. toggle_implementation: CourseWaffleFlag
|
||||
# .. toggle_default: False
|
||||
# .. toggle_description: Waffle flag to enable the notify all learners on discussion post
|
||||
# .. toggle_use_cases: open_edx
|
||||
# .. toggle_creation_date: 2025-06-11
|
||||
# .. toggle_warning: When the flag is ON, notification to all learners feature is enabled on discussion post.
|
||||
# .. toggle_tickets: INF-1917
|
||||
ENABLE_NOTIFY_ALL_LEARNERS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_post_notify_all_learners', __name__)
|
||||
|
||||
@@ -26,7 +26,7 @@ NOTIFICATION_CHANNELS = ['web', 'push', 'email']
|
||||
ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS = ['email_cadence']
|
||||
|
||||
# Update this version when there is a change to any course specific notification type or app.
|
||||
COURSE_NOTIFICATION_CONFIG_VERSION = 13
|
||||
COURSE_NOTIFICATION_CONFIG_VERSION = 14
|
||||
|
||||
|
||||
def get_course_notification_preference_config():
|
||||
|
||||
@@ -311,7 +311,10 @@ class TestVisibilityFilter(unittest.TestCase):
|
||||
'core': {'web': True, 'push': True, 'email': True, 'email_cadence': 'Daily'},
|
||||
'content_reported': {'web': True, 'push': True, 'email': True, 'email_cadence': 'Daily'},
|
||||
'new_question_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'},
|
||||
'new_discussion_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'}
|
||||
'new_discussion_post': {'web': False, 'push': False, 'email': False, 'email_cadence': 'Daily'},
|
||||
'new_instructor_all_learners_post': {
|
||||
'web': True, 'push': False, 'email': False, 'email_cadence': 'Daily'
|
||||
}
|
||||
},
|
||||
'core_notification_types': [
|
||||
'new_response', 'comment_on_followed_post',
|
||||
|
||||
@@ -4,9 +4,11 @@ Utils function for notifications app
|
||||
import copy
|
||||
from typing import Dict, List, Set
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment
|
||||
from openedx.core.djangoapps.django_comment_common.models import Role
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
|
||||
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS
|
||||
from openedx.core.lib.cache_utils import request_cached
|
||||
|
||||
|
||||
@@ -132,12 +134,21 @@ def remove_preferences_with_no_access(preferences: dict, user) -> dict:
|
||||
user=user,
|
||||
course_id=preferences['course_id']
|
||||
).values_list('role', flat=True)
|
||||
preferences['notification_preference_config'] = filter_out_visible_notifications(
|
||||
|
||||
user_preferences = filter_out_visible_notifications(
|
||||
user_preferences,
|
||||
notifications_with_visibility_settings,
|
||||
user_forum_roles,
|
||||
user_course_roles
|
||||
)
|
||||
|
||||
course_key = CourseKey.from_string(preferences['course_id'])
|
||||
discussion_config = user_preferences.get('discussion', {})
|
||||
notification_types = discussion_config.get('notification_types', {})
|
||||
|
||||
if notification_types and not ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key):
|
||||
notification_types.pop('new_instructor_all_learners_post', None)
|
||||
|
||||
return preferences
|
||||
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ from openedx.core.djangoapps.notifications.serializers import add_info_to_notifi
|
||||
from openedx.core.djangoapps.user_api.models import UserPreference
|
||||
|
||||
from .base_notification import COURSE_NOTIFICATION_APPS
|
||||
from .config.waffle import ENABLE_NOTIFICATIONS
|
||||
from .config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFY_ALL_LEARNERS
|
||||
from .events import (
|
||||
notification_preference_update_event,
|
||||
notification_preferences_viewed_event,
|
||||
@@ -603,13 +603,23 @@ class AggregatedNotificationPreferences(APIView):
|
||||
notification_configs = aggregate_notification_configs(
|
||||
notification_configs
|
||||
)
|
||||
course_ids = notification_preferences.values_list('course_id', flat=True)
|
||||
|
||||
filter_out_visible_preferences_by_course_ids(
|
||||
request.user,
|
||||
notification_configs,
|
||||
notification_preferences.values_list('course_id', flat=True),
|
||||
course_ids,
|
||||
)
|
||||
|
||||
notification_preferences_viewed_event(request)
|
||||
notification_configs = add_info_to_notification_config(notification_configs)
|
||||
|
||||
discussion_config = notification_configs.get('discussion', {})
|
||||
notification_types = discussion_config.get('notification_types', {})
|
||||
|
||||
if not any(ENABLE_NOTIFY_ALL_LEARNERS.is_enabled(course_key) for course_key in course_ids):
|
||||
notification_types.pop('new_instructor_all_learners_post', None)
|
||||
|
||||
return Response({
|
||||
'status': 'success',
|
||||
'message': 'Notification preferences retrieved',
|
||||
|
||||
Reference in New Issue
Block a user