Skip to content

feat: Add notify all learners option for discussion post #36922

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

Merged
merged 2 commits into from
Jun 27, 2025
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
16 changes: 12 additions & 4 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@
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()
Expand Down Expand Up @@ -333,6 +334,8 @@ def _format_datetime(dt):
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,
Expand All @@ -358,8 +361,8 @@ def _format_datetime(dt):
}),
"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),
Expand All @@ -372,6 +375,9 @@ def _format_datetime(dt):
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
),
}


Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,18 @@ def send_response_endorsed_notification(self):
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()
Expand Down
15 changes: 13 additions & 2 deletions lms/djangoapps/discussion/rest_api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
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
Expand All @@ -17,7 +20,7 @@

@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
"""
Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions lms/djangoapps/discussion/rest_api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ def test_basic(self):
'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(
Expand Down Expand Up @@ -1918,7 +1919,9 @@ def test_basic(self, mock_emit):
"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",
Expand Down Expand Up @@ -1984,7 +1987,9 @@ def test_basic_in_blackout_period_with_user_access(self, mock_emit):

_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",
Expand Down Expand Up @@ -2056,7 +2061,9 @@ def test_title_truncation(self, mock_emit):
"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'
Expand Down
49 changes: 33 additions & 16 deletions lms/djangoapps/discussion/rest_api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
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

Expand Down Expand Up @@ -190,29 +190,46 @@ def test_not_authenticated(self):
"""

@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'),
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/rest_api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ def test_basic(self):
"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
}
)

Expand Down
23 changes: 23 additions & 0 deletions lms/djangoapps/discussion/rest_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

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,
Expand Down Expand Up @@ -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)
5 changes: 4 additions & 1 deletion lms/djangoapps/discussion/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions openedx/core/djangoapps/notifications/base_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,24 @@
'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 = {
Expand Down
10 changes: 10 additions & 0 deletions openedx/core/djangoapps/notifications/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,13 @@
# .. 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__)
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/notifications/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
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():
Expand Down
5 changes: 4 additions & 1 deletion openedx/core/djangoapps/notifications/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ def setUp(self):
'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',
Expand Down
15 changes: 13 additions & 2 deletions openedx/core/djangoapps/notifications/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
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


Expand Down Expand Up @@ -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


Expand Down
Loading
Loading